New API for downloading LDML files from projects that allow sharing WS data #1309
Conversation
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
|
Unfortunately it looks as though we won't be able to use ZipArchive to add files to the output stream asynchronously as they come in: we'd probably run into this issue where ZipArchive tries to write the end-of-zipfile directory synchronously when it's disposed, but Kestrel doesn't allow synchronous writes to its HTTP output stream. So we might have to extract everything, then send the zip file separately. Which means that the HTTP connection might time out, so it's possible we'll have to make two API endpoints, one to prepare the zip file and one to download it. Testing required. |
Will return 403 Forbidden if project does not allow sharing ws data with SLDR. Will also return same 403 Forbidden error code if project does not exist, to avoid possibly leaking project codes. If project exists and allows data sharing, command will return a zipfile containing CachedSettings/WritingSystems/*.ldml from the tip revision.
rmunn
force-pushed
the
feat/api-for-ldml-zips
branch
from
8888014 to
ae60963
Compare
|
Rebased on top of current develop since it's been a month since this was started, and a lot has changed on the develop branch in that time. |
Alas, this fails (when a ProjectController method is added to call PrepareLdmlZip) with "System.InvalidOperationException: Synchronous operations are disallowed. Call WriteAsync or set AllowSynchronousIO to true instead." We'll have to change this to prepare the entire zip file first, then send it.
This is less efficient, but the only method that actually works given the current state of the ZipFile / ZipArchive code in .NET.
|
Now working; marking as ready for review. Not ready to merge yet, as there are several discussion items (marked with TODO comments) in this PR, such as what the permission structure should be. But it's tested and working. To test locally, you'll need to get a project with
If all went well, the zipfile should contain a folder named for a project ID at the root. If you get a 22-byte zip file that's empty, then something went wrong. |
Can safely add an optional `delay` parameter to LexJob instead
Zip file now has timestamp in name, so this API can be called multiple times within a 4-hour period without interfering with other calls.
|
@hahn-kev wrote:
All comments addressed, and commit e7b98d8 sets it up to return 404 instead of an empty zip, as well as putting a UTC timestamp into the filename so that it's more obvious when the API call was run. This is ready for re-review. |
Fixes #1306.
To test locally, you'll need to get a project with
addToSldr="true"in its writing system configuration. None of our default test projects have that, but you can add that with the following method:kubectl --context=docker-desktop exec -it deploy/hg -- bashcd /var/hg/repos/s/sena-3or/e/elawa-dev-flexhg co tipto populate the repo's working directoryCachedSettings/SharedSettings/LexiconSettings.plsxfile<WritingSystems>line to<WritingSystems addToSldr="true">hg commit -m "Add to SLDR"hg config --editto set your usernamehg co nullto empty the working directory now that the commit is in the repoIf all went well, the zipfile should contain a folder named for a project ID at the root. If you get a 22-byte zip file that's empty, then something went wrong.
Currently the directory structure for each project looks like this:
{project ID}/CachedSettings/WritingSystemStore/lots of .ldml filesDo we want to change that to just this?
{project ID}/lots of .ldml filesDiscussion items
CachedSettings/WritingSystemStore/?